Skip to content

feat: update several path for image.tag #793

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 6 commits into
base: master-annotation-based
Choose a base branch
from
Open

feat: update several path for image.tag #793

wants to merge 6 commits into from

Conversation

@Tchoupinax
Copy link

Hello folks!

I made this improvement to handle a case we have at work. Let's explain it.

In the current case, we provide a path for the image tag with the annotation .helm.image-tag. It says where to update the tag for this argoCD application.

Now, I have one ArgoCD application with two deployements, different with a suffix. Let's call them a and b. So my helm-chart values are prefixed by the alias of each app.

a:
  image:
    tag: v1.0.0

b:
  image:
    tag: v1.0.0

I want to update both at the same time, the lifecycle is identical. This merge request allows to do it, by adding several paths as value separated by a comma. Then, I can provide

xxxx.helm.image-tag=a.image.tag,b.image.tag

A unit test has been added about tag and name and moreover, I handled every space to ensure it won't have any bug for a mistype.

Finally, I build the customized image and tested, all is OK for me.
I hope all is clear!

Thank you for your work, I love this project.
Cheers!

@codecov-commenter
Copy link

codecov-commenter commented Jul 22, 2024 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.70%. Comparing base (b179e94) to head (8e9e3cd).

Additional details and impacted files 
@@             Coverage Diff             @@
##           master     #793       +/-   ##
===========================================
+ Coverage   70.85%   82.70%   +11.84%     
===========================================
  Files          45       22       -23     
  Lines        5178     1954     -3224     
===========================================
- Hits         3669     1616     -2053     
+ Misses       1344      282     -1062     
+ Partials      165       56      -109

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@chengfang
Copy link
Collaborator

@Tchoupinax thanks for proposing it. I'll look into it.

@Tchoupinax
Copy link
Author

Hello @chengfang,
Could you have time to give a look again? 🙏

@chengfang chengfang added the enhancement New feature or request label Oct 21, 2024
chengfang
chengfang reviewed Oct 21, 2024
View reviewed changes
@chengfang
Copy link
Collaborator

Some other places assume that image.name and image.spec represent single-path value, and we should think about if it makes sense for those places to also take multi-path values. For instance,

argocd-image-updater/pkg/argocd/update.go

Line 449 in 4f21ade

helmAnnotationParamName, helmAnnotationParamVersion := getHelmParamNamesFromAnnotation(app.Annotations, c)

argocd-image-updater/pkg/argocd/update.go

Line 473 in 4f21ade

helmParamName := getHelmParam(appSource.Helm.Parameters, helmAnnotationParamName)

chengfang
chengfang reviewed Oct 21, 2024
View reviewed changes
@Tchoupinax
Copy link
Author

I will give a look but maybe I need help on your last point (checking if having two paths is OK everywhere)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chengfang chengfang chengfang left review comments

@jannfis jannfis Awaiting requested review from jannfis jannfis is a code owner

@pasha-codefresh pasha-codefresh Awaiting requested review from pasha-codefresh pasha-codefresh is a code owner

@ishitasequeira ishitasequeira Awaiting requested review from ishitasequeira ishitasequeira is a code owner

@dkarpele dkarpele Awaiting requested review from dkarpele dkarpele is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Tchoupinax @codecov-commenter @chengfang